[clang-tidy] Add check 'bugprone-invalid-enum-default-initialization' #136823

balazske · 2025-04-23T07:45:00Z

No description provided.

llvmbot · 2025-04-23T07:45:36Z

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Balázs Kéri (balazske)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/136823.diff

6 Files Affected:

(modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
(modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
(added) clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp (+114)
(added) clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h (+32)
(added) clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst (+50)
(added) clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp (+116)

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index b780a85bdf3fe..893dd140c92b0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -38,6 +38,7 @@
 #include "IncorrectRoundingsCheck.h"
 #include "InfiniteLoopCheck.h"
 #include "IntegerDivisionCheck.h"
+#include "InvalidEnumDefaultInitializationCheck.h"
 #include "LambdaFunctionNameCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
@@ -164,6 +165,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop");
     CheckFactories.registerCheck<IntegerDivisionCheck>(
         "bugprone-integer-division");
+    CheckFactories.registerCheck<InvalidEnumDefaultInitializationCheck>(
+        "bugprone-invalid-enum-default-initialization");
     CheckFactories.registerCheck<LambdaFunctionNameCheck>(
         "bugprone-lambda-function-name");
     CheckFactories.registerCheck<MacroParenthesesCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e310ea9c94543..bf142575becfb 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -30,6 +30,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   InaccurateEraseCheck.cpp
   IncorrectEnableIfCheck.cpp
   IncorrectEnableSharedFromThisCheck.cpp
+  InvalidEnumDefaultInitializationCheck.cpp
   UnintendedCharOstreamOutputCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SuspiciousStringviewDataUsageCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp
new file mode 100644
index 0000000000000..152c2468cd121
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp
@@ -0,0 +1,114 @@
+//===--- InvalidEnumDefaultInitializationCheck.cpp - clang-tidy -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "InvalidEnumDefaultInitializationCheck.h"
+// #include "../utils/Matchers.h"
+// #include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) {
+  const EnumDecl *Definition = Node.getDefinition();
+  return Definition && Node.isComplete() &&
+         std::none_of(Definition->enumerator_begin(),
+                      Definition->enumerator_end(),
+                      [](const EnumConstantDecl *Value) {
+                        return Value->getInitVal().isZero();
+                      });
+}
+
+AST_MATCHER(Expr, isEmptyInit) {
+  if (isa<CXXScalarValueInitExpr>(&Node))
+    return true;
+  if (isa<ImplicitValueInitExpr>(&Node))
+    return true;
+  if (const auto *Init = dyn_cast<InitListExpr>(&Node))
+    return Init->getNumInits() == 0;
+  return false;
+}
+
+} // namespace
+
+InvalidEnumDefaultInitializationCheck::InvalidEnumDefaultInitializationCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
+bool InvalidEnumDefaultInitializationCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}
+
+void InvalidEnumDefaultInitializationCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(
+      expr(isEmptyInit(),
+           hasType(hasUnqualifiedDesugaredType(enumType(hasDeclaration(
+               enumDecl(isCompleteAndHasNoZeroValue()).bind("enum"))))))
+          .bind("expr"),
+      this);
+}
+
+void InvalidEnumDefaultInitializationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *InitExpr = Result.Nodes.getNodeAs<Expr>("expr");
+  const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum");
+  if (!InitExpr || !Enum)
+    return;
+
+  ASTContext &ACtx = Enum->getASTContext();
+  SourceLocation Loc = InitExpr->getExprLoc();
+  if (Loc.isInvalid()) {
+    if (isa<ImplicitValueInitExpr>(InitExpr) || isa<InitListExpr>(InitExpr)) {
+      DynTypedNodeList Parents = ACtx.getParents(*InitExpr);
+      if (Parents.empty())
+        return;
+
+      if (const auto *Ctor = Parents[0].get<CXXConstructorDecl>()) {
+        // Try to find member initializer with the found expression and get the
+        // source location from it.
+        CXXCtorInitializer *const *CtorInit = std::find_if(
+            Ctor->init_begin(), Ctor->init_end(),
+            [InitExpr](const CXXCtorInitializer *Init) {
+              return Init->isMemberInitializer() && Init->getInit() == InitExpr;
+            });
+        if (!CtorInit)
+          return;
+        Loc = (*CtorInit)->getLParenLoc();
+      } else if (const auto *InitList = Parents[0].get<InitListExpr>()) {
+        // The expression may be implicitly generated for an initialization.
+        // Search for a parent initialization list with valid source location.
+        while (InitList->getExprLoc().isInvalid()) {
+          DynTypedNodeList Parents = ACtx.getParents(*InitList);
+          if (Parents.empty())
+            return;
+          InitList = Parents[0].get<InitListExpr>();
+          if (!InitList)
+            return;
+        }
+        Loc = InitList->getExprLoc();
+      }
+    }
+    // If still not found a source location, omit the warning.
+    // FIXME: All such cases should be fixed to make the checker more precise.
+    if (Loc.isInvalid())
+      return;
+  }
+  diag(Loc, "enum value of type %0 initialized with invalid value of 0, "
+            "enum doesn't have a zero-value enumerator")
+      << Enum;
+  diag(Enum->getLocation(), "enum is defined here", DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h
new file mode 100644
index 0000000000000..fa5d068f2ae32
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h
@@ -0,0 +1,32 @@
+//===--- InvalidEnumDefaultInitializationCheck.h - clang-tidy -*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detect default initialization (to 0) of variables with `enum` type where
+/// the enum has no enumerator with value of 0.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/.html
+class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck {
+public:
+  InvalidEnumDefaultInitializationCheck(StringRef Name,
+                                        ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst
new file mode 100644
index 0000000000000..634dec2aecd25
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - bugprone-invalid-enum-default-initialization
+
+bugprone-invalid-enum-default-initialization
+============================================
+
+Detect default initialization (to 0) of variables with `enum` type where
+the enum has no enumerator with value of 0.
+
+In C++ a default initialization is performed if a variable is initialized with
+initializer list or in other implicit ways, and no value is specified at the
+initialization. In such cases the value 0 is used for the initialization.
+This also applies to enumerations even if it does not have an enumerator with
+value 0. In this way a variable with the enum type may contain initially an
+invalid value (if the program expects that it contains only the listed
+enumerator values).
+
+The checker emits a warning only if an enum variable is default-initialized
+(contrary to not initialized) and the enum type does not have an enumerator with
+value of 0. The enum type can be scoped or non-scoped enum.
+
+.. code-block:: c++
+
+  enum class Enum1: int {
+    A = 1,
+    B
+  };
+
+  enum class Enum0: int {
+    A = 0,
+    B
+  };
+
+  void f() {
+    Enum1 X1{}; // warn: 'X1' is initialized to 0
+    Enum1 X2 = Enum1(); // warn: 'X2' is initialized to 0
+    Enum1 X3; // no warning: 'X3' is not initialized
+    Enum0 X4{}; // no warning: type has an enumerator with value of 0
+  }
+
+  struct S1 {
+    Enum1 A;
+    S(): A() {} // warn: 'A' is initialized to 0
+  };
+
+  struct S2 {
+    int A;
+    Enum1 B;
+  };
+
+  S2 VarS2{}; // warn: member 'B' is initialized to 0
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp
new file mode 100644
index 0000000000000..857093c2239cd
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy -std=c++17 %s bugprone-invalid-enum-default-initialization %t
+
+enum class Enum0: int {
+  A = 0,
+  B
+};
+
+enum class Enum1: int {
+  A = 1,
+  B
+};
+
+enum Enum2 {
+  Enum_A = 4,
+  Enum_B
+};
+
+Enum0 E0_1{};
+Enum0 E0_2 = Enum0();
+Enum0 E0_3;
+Enum0 E0_4{0};
+Enum0 E0_5{Enum0::A};
+Enum0 E0_6{Enum0::B};
+
+Enum1 E1_1{};
+// CHECK-NOTES: :[[@LINE-1]]:11: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :8:12: note: enum is defined here
+Enum1 E1_2 = Enum1();
+// CHECK-NOTES: :[[@LINE-1]]:14: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :8:12: note: enum is defined here
+Enum1 E1_3;
+Enum1 E1_4{0};
+Enum1 E1_5{Enum1::A};
+Enum1 E1_6{Enum1::B};
+
+Enum2 E2_1{};
+// CHECK-NOTES: :[[@LINE-1]]:11: warning: enum value of type 'Enum2' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :13:6: note: enum is defined here
+Enum2 E2_2 = Enum2();
+// CHECK-NOTES: :[[@LINE-1]]:14: warning: enum value of type 'Enum2' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :13:6: note: enum is defined here
+
+void f1() {
+  static Enum1 S; // FIMXE: warn for this?
+  Enum1 A;
+  Enum1 B = Enum1();
+  // CHECK-NOTES: :[[@LINE-1]]:13: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  int C = int();
+}
+
+void f2() {
+  Enum1 A{};
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  Enum1 B = Enum1();
+  // CHECK-NOTES: :[[@LINE-1]]:13: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  Enum1 C[5] = {{}};
+  // CHECK-NOTES: :[[@LINE-1]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  Enum1 D[5] = {}; // FIMXE: warn for this?
+}
+
+struct S1 {
+  Enum1 E_1{};
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  Enum1 E_2 = Enum1();
+  // CHECK-NOTES: :[[@LINE-1]]:15: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  Enum1 E_3;
+  Enum1 E_4;
+  Enum1 E_5;
+
+  S1() :
+    E_3{},
+    // CHECK-NOTES: :[[@LINE-1]]:8: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+    // CHECK-NOTES: :8:12: note: enum is defined here
+    E_4(),
+    // CHECK-NOTES: :[[@LINE-1]]:8: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+    // CHECK-NOTES: :8:12: note: enum is defined here
+    E_5{Enum1::B}
+  {}
+};
+
+struct S2 {
+  Enum0 X;
+  Enum1 Y;
+  Enum2 Z;
+};
+
+struct S3 {
+  S2 X;
+  int Y;
+};
+
+struct S4 : public S3 {
+  int Z;
+};
+
+S2 VarS2{};
+// CHECK-NOTES: :[[@LINE-1]]:9: warning: enum value of type 'Enum1' initialized with invalid value of 0
+// CHECK-NOTES: :8:12: note: enum is defined here
+// CHECK-NOTES: :[[@LINE-3]]:9: warning: enum value of type 'Enum2' initialized with invalid value of 0
+// CHECK-NOTES: :13:6: note: enum is defined here
+S3 VarS3{};
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0
+// CHECK-NOTES: :8:12: note: enum is defined here
+// CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0
+// CHECK-NOTES: :13:6: note: enum is defined here
+S4 VarS4{};
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0
+// CHECK-NOTES: :8:12: note: enum is defined here
+// CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0
+// CHECK-NOTES: :13:6: note: enum is defined here

firewave · 2025-04-23T08:39:28Z

See also
https://reviews.llvm.org/D144036
#58127

PiotrZSL · 2025-04-23T08:58:57Z

clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp

@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy -std=c++17 %s bugprone-invalid-enum-default-initialization %t
+


Add test with enum forward declaration only, and with enum without enumerators.
For those there should be no requirement in initialization.

These are added now.

vbvictor · 2025-04-23T14:02:50Z

I wonder if behavior of this check should be an extension to optin.core.EnumCastOutOfRange. Sure, we don't have static_cast here but it's still considered as "cast out of range"? Maybe it's better to add an option to EnumCastOutOfRange like AnalyzeInitialization and don't write a whole new check?

EugeneZelenko

Please add Release Notes entry.

EugeneZelenko · 2025-04-23T14:33:05Z

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp

+  if (!InitExpr || !Enum)
+    return;
+
+  ASTContext &ACtx = Enum->getASTContext();


const? Same for Parents below.

getParents requires it to be non-const.

EugeneZelenko · 2025-04-23T14:33:36Z

clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst

+bugprone-invalid-enum-default-initialization
+============================================
+
+Detect default initialization (to 0) of variables with `enum` type where


Suggested change

Detect default initialization (to 0) of variables with `enum` type where

Detect default initialization (to 0) of variables with ``enum`` type where

firewave · 2025-04-23T17:20:01Z

There's also a check in UBSAN which detects assignment of invalid values during run-time. So there's logic is various other places regarding this.

balazske · 2025-04-24T08:29:52Z

I wonder if behavior of this check should be an extension to optin.core.EnumCastOutOfRange. Sure, we don't have static_cast here but it's still considered as "cast out of range"? Maybe it's better to add an option to EnumCastOutOfRange like AnalyzeInitialization and don't write a whole new check?

This check is implemented as an AST based check that is different from the static analyzer check, even if there is a similarity in the found problems. (It can be possible to add this check as not AST-based static analyzer check but the results would be likely not as exact.) Turning on an option in an existing checker is not much different than turning on another checker.

EugeneZelenko · 2025-04-24T14:38:21Z

clang-tools-extra/docs/ReleaseNotes.rst

  Finds potentially erroneous calls to ``reset`` method on smart pointers when
  the pointee type also has a ``reset`` method.

+- New :doc:`bugprone-invalid-enum-default-initialization


Please keep alphabetical order (by check name) in this list.

EugeneZelenko · 2025-04-24T14:38:38Z

clang-tools-extra/docs/ReleaseNotes.rst

+- New :doc:`bugprone-invalid-enum-default-initialization
+  <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
+
+  Detect default initialization (to 0) of variables with ``enum`` type where


Suggested change

Detect default initialization (to 0) of variables with ``enum`` type where

Detects default initialization (to 0) of variables with ``enum`` type where

EugeneZelenko · 2025-04-24T14:38:49Z

clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst

 ============================================

-Detect default initialization (to 0) of variables with `enum` type where
+Detect default initialization (to 0) of variables with ``enum`` type where


Suggested change

Detect default initialization (to 0) of variables with ``enum`` type where

Detects default initialization (to 0) of variables with ``enum`` type where

balazske · 2025-05-09T08:33:05Z

I have fixed the review issues.

HerrCai0907

LGTM.

HerrCai0907 · 2025-05-16T03:36:44Z

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp

+bool InvalidEnumDefaultInitializationCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}


Does this check also valuable for C?

Check was improved to handle initializer lists that can be used in C code too.

balazske · 2025-05-29T07:03:40Z

The check can now warn for (default) initializations that are recursively inside a type structure (members of structs, elements of arrays). In these cases it can look difficult to find the member or place where the value with enum type exists. The check now only makes a single warning for the initialization but does not add notes to find the place of the enum value.

gamesh411

LGTM, minor comments/questions inline.

gamesh411 · 2025-07-24T23:15:16Z

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp

+    return false;
+  }
+  bool VisitArrayType(const ArrayType *T) {
+    return Visit(T->getElementType()->getUnqualifiedDesugaredType());


Is calling getUnqualifiedDesugaredType necessary in VisitArrayType, VisitConstantArrayType and the field visitation of VisitRecordType? If they always call forward to VisitType, is it not enough to have the desugaring done only in VisitType? Feel free to correct me if I am wrong about this.

It looks redundant, I have removed getUnqualifiedDesugaredType when Visit is called (but getTypePtr is needed instead).

gamesh411 · 2025-07-24T23:25:58Z

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h

+                                        ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};


This is relevant for C, C++ and Objective-C languages as well, so I think you are right to not override the isLanguageVersionSupported method here.

isLanguageVersionSupported was already removed.

gamesh411 · 2025-07-24T23:29:47Z

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp

+  ASTContext &ACtx = Enum->getASTContext();
+  SourceLocation Loc = InitExpr->getExprLoc();
+  if (Loc.isInvalid()) {
+    if (isa<ImplicitValueInitExpr>(InitExpr) || isa<InitListExpr>(InitExpr)) {


I think isa<>() now supports variadic args, so we could use:
isa<ImplicitValueInitExpr, InitListExpr>(InitExpr)

EugeneZelenko · 2025-07-24T23:36:29Z

clang-tools-extra/docs/ReleaseNotes.rst

  pointer and store it as class members without handle the copy and move
  constructors and the assignments.

+- New :doc:`bugprone-invalid-enum-default-initialization


Please rebase from main.

I do not use rebase because it may remove commits and make comparison difficult. Instead a merge from main was done.

Merge complicates history and make it non-linear.

Unfortunately the github pull request UI gets confused when commits are rebased or amended on the branch that acts as the "source" of the PR. (IIRC inline comments will get lost, but there may be other problems as well.) For this reason I think it's prudent to use merge commits during the lifetime of a github review.

Note that when the PR is accepted all the commits on it (including the merge commits) will be squashed into a single commit that gets rebased onto the main branch -- so there is no danger of complicating the canonical history of the project. (And the local/internal history of a single PR is small enough that it will remain understandable even if there are merge commits from main.)

LLVM documentation (https://llvm.org/docs/GitHub.html) actually tells that we should rebase the PR after the review is finished, force push a final commit and merge this on github. (But rebase can involve changes that should be reviewed?) Comparing changes between PR commits (like a diff only of the last update) seems to be not possible after rebase. "Squash and merge" does work (if there is no conflict) and leaves a single (non-merge) commit in the main branch.

LLVM documentation (https://llvm.org/docs/GitHub.html) actually tells that we should rebase the PR after the review is finished, force push a final commit and merge this on github.

The documentation primarily suggest using the "Squash and merge" button and only offers this "Merge using the GitHub command line interface" workflow (that you appear to summarize) as a secondary alternative. Also note that there is almost no difference between using "Squash and merge" and this CLI workflow.

(But rebase can involve changes that should be reviewed?)

Yes, some rebases can involve nontrivial changes, but the intended workflow is that:

if the change is not yet compatible with the main branch (a rebase would involve nontrivial changes), then merge the main branch to the PR branch, add the necessary correction commits and review them;

once the change is complete and compatible with the main branch (i.e. it can be trivially rebased), it should be squashed and rebased (either by the slightly misnamed "Squash and merge" button, or the equivalent CLI workflow) -- and in this situation the rebase is trivial.

Comparing changes between PR commits (like a diff only of the last update) seems to be not possible after rebase.

Yes, that's a significant reason why rebases during the lifetime of the PR should be avoided.

"Squash and merge" does work (if there is no conflict) and leaves a single (non-merge) commit in the main branch.

Yes, it's a bit unfortunate that they called it "Squash and merge" instead of "Squash and rebase" (which would accurately describe its effects) -- but it works correctly.

EugeneZelenko · 2025-07-30T14:03:15Z

clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst

+initializer list or in other implicit ways, and no value is specified at the
+initialization. In such cases the value 0 is used for the initialization.
+This also applies to enumerations even if it does not have an enumerator with
+value 0. In this way a variable with the enum type may contain initially an


Suggested change

value 0. In this way a variable with the enum type may contain initially an

value 0. In this way a variable with the ``enum`` type may contain initially an

EugeneZelenko · 2025-07-30T14:04:19Z

clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst

+The checker emits a warning only if an enum variable is default-initialized
+(contrary to not initialized) and the enum type does not have an enumerator with
+value of 0. The enum type can be scoped or non-scoped enum. Unions are not
+handled by the check (if it contains a member of enum type).


Suggested change

The checker emits a warning only if an enum variable is default-initialized

(contrary to not initialized) and the enum type does not have an enumerator with

value of 0. The enum type can be scoped or non-scoped enum. Unions are not

handled by the check (if it contains a member of enum type).

The check emits a warning only if an ``enum`` variable is default-initialized

(contrary to not initialized) and the ``enum`` type does not have an enumerator with

value of 0. The ``enum`` type can be scoped or non-scoped ``enum``. Unions are not

handled by the check (if it contains a member of ``enum`` type).

[clang-tidy] Add check 'bugprone-invalid-enum-default-initialization'

4ce7497

llvmbot added clang-tools-extra clang-tidy labels Apr 23, 2025

balazske requested review from Discookie and NagyDonat April 23, 2025 08:51

PiotrZSL reviewed Apr 23, 2025

View reviewed changes

EugeneZelenko requested review from HerrCai0907 and carlosgalvezp April 23, 2025 14:31

EugeneZelenko reviewed Apr 23, 2025

View reviewed changes

small code fixes, added some tests, fixed doc

e5744a8

EugeneZelenko reviewed Apr 24, 2025

View reviewed changes

balazske added 2 commits April 25, 2025 10:32

documentation fixes

6920e31

updated list.rst

9620955

balazske requested review from EugeneZelenko and PiotrZSL May 9, 2025 08:33

HerrCai0907 approved these changes May 16, 2025

View reviewed changes

balazske added 2 commits May 23, 2025 10:05

Merge branch 'main' into tidy_invalid-default-enum-init

cea08d8

improvements for initialization lists

b726e03

added comment about functionality

05f3e6e

EugeneZelenko requested a review from vbvictor May 29, 2025 13:51

Merge branch 'main' into tidy_invalid-default-enum-init

c7fa29f

balazske requested a review from HerrCai0907 July 1, 2025 07:52

gamesh411 approved these changes Jul 24, 2025

View reviewed changes

EugeneZelenko reviewed Jul 24, 2025

View reviewed changes

balazske added 3 commits July 25, 2025 17:27

Merge branch 'main' into tidy_invalid-default-enum-init

d164305

code simplifications

26c0a34

fixed build error

c7615e0

EugeneZelenko reviewed Jul 30, 2025

View reviewed changes

fixed documentation issues

245d7b9

balazske merged commit 6f2cf6b into llvm:main Jul 31, 2025
10 checks passed

balazske deleted the tidy_invalid-default-enum-init branch July 31, 2025 07:00

		@@ -0,0 +1,116 @@
		// RUN: %check_clang_tidy -std=c++17 %s bugprone-invalid-enum-default-initialization %t

	Detect default initialization (to 0) of variables with `enum` type where
	Detect default initialization (to 0) of variables with ``enum`` type where

	Detect default initialization (to 0) of variables with ``enum`` type where
	Detects default initialization (to 0) of variables with ``enum`` type where

	value 0. In this way a variable with the enum type may contain initially an
	value 0. In this way a variable with the ``enum`` type may contain initially an

[clang-tidy] Add check 'bugprone-invalid-enum-default-initialization' #136823

[clang-tidy] Add check 'bugprone-invalid-enum-default-initialization' #136823

Uh oh!

Conversation

balazske commented Apr 23, 2025

Uh oh!

llvmbot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firewave commented Apr 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbvictor commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EugeneZelenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

firewave commented Apr 23, 2025

Uh oh!

balazske commented Apr 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balazske commented May 9, 2025

Uh oh!

HerrCai0907 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balazske commented May 29, 2025

Uh oh!

gamesh411 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

llvmbot commented Apr 23, 2025 •

edited

Loading

vbvictor commented Apr 23, 2025 •

edited

Loading